Closed Bug 1803607 Opened 2 years ago Closed 2 months ago

Make about:logging work on mobile

Categories

(Core :: XPCOM, enhancement)

Product:
Core
Component:
XPCOM
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
139 Branch
Tracking Flags:
Tracking Status
firefox139 --- fixed

People

(Reporter: padenot, Assigned: julienw)

References

(Blocks 2 open bugs, Regressed 1 open bug)

Details

(Whiteboard: [fxp])

Attachments

(9 files, 3 obsolete files)

Bug 1803607 - Convert aboutLogging.js to a JS module, and other minor changes r=canaltinova
48 bytes, text/x-phabricator-request
Details | Review
Bug 1803607 - [about:logging] Avoid calling desktop-specific functions on android r=canaltinova
48 bytes, text/x-phabricator-request
Details | Review
Bug 1803607 - [about:logging] Provide a separate UI for Android r=canaltinova
48 bytes, text/x-phabricator-request
Details | Review
Bug 1803607 - Move the about:logging code to its own subdirectory r=canaltinova
48 bytes, text/x-phabricator-request
Details | Review
Bug 1803607 - [about:logging] Add a test to cover the new functionality r=canaltinova
48 bytes, text/x-phabricator-request
Details | Review
Bug 1803607 - [about:logging] Handle uploading errors r=canaltinova
48 bytes, text/x-phabricator-request
Details | Review
Bug 1803607 - Allow using https: hosts for connect-src in CSP validation
48 bytes, text/x-phabricator-request
Details | Review
Screencast_20250401_160515.webm
970.27 KB, video/webm
Details
Bug 1803607 - Localize about:logging on Android too r=flod
48 bytes, text/x-phabricator-request
Details | Review

We need to use proper branding instead of duplicating -profiler-brand-name, and have CSS that's more suited to a mobile device.

There's no background.jsm.js on mobile so there is a bit of work needed. We might be able to call into the profiler via js directly or something.

Summary: Make about:logging work better on mobile → Make about:logging work
Summary: Make about:logging work → Make about:logging work on mobile
Depends on: 1842589
Duplicate of this bug: 1896982
Attachment #9473408 - Attachment description: WIP: Bug 1803607 - Minor changes in aboutLogging.js r=canaltinova → WIP: Bug 1803607 - Convert aboutLogging.js to a JS module, and other minor changes r=canaltinova

This will make it possible to add some utilitary javascript files
without cluttering the main directory toolkit/content.

Depends on: 1957013
Assignee: padenot → felash
Depends on: 1957016

Comment on attachment 9473923 [details]
WIP: Bug 1803607 - Convert recording-utils.js to a ES module r=canaltinova

Revision D242689 was moved to bug 1957016. Setting attachment 9473923 [details] to obsolete.

Attachment #9473923 - Attachment is obsolete: true

Comment on attachment 9473924 [details]
WIP: Bug 1803607 - Split background.sys.mjs into smaller files r=canaltinova

Revision D242690 was moved to bug 1957016. Setting attachment 9473924 [details] to obsolete.

Attachment #9473924 - Attachment is obsolete: true

Comment on attachment 9474823 [details]
WIP: Bug 1803607 - Move the capturing profile part to recording-utils.sys.mjs r=canaltinova

Revision D243184 was moved to bug 1957016. Setting attachment 9474823 [details] to obsolete.

Attachment #9474823 - Attachment is obsolete: true
Attachment #9474824 - Attachment description: WIP: Bug 1803607 - Provide a separate UI for Android r=canaltinova → WIP: Bug 1803607 - [about:logging] Provide a separate UI for Android r=canaltinova
Whiteboard: [fxp]

Hey Tom,

In this work I want to be able to upload a Firefox profile from the about:logging page. The upload is a POST to https://api.profiler.firefox.com, so I added this origin to a connect-src entry in the CSP. But this triggers the crash in https://searchfox.org/mozilla-central/rev/5fb48bf50516ed2529d533e5dfe49b4752efb8b8/dom/security/nsContentSecurityUtils.cpp#1847.

What do you suggest to make this work?
Thanks!

Flags: needinfo?(tschuster)

Ah I see with Tom's other email that he's blocked his needinfo requests.

Hey other Tom :-) do you have any recommendation about my previous question in comment 16?

Flags: needinfo?(tschuster) → needinfo?(tom)

You can use connect-src https: and add the URL about:logging to the sConnectSrcHttpsAllowList. I can also provide a patch that would make it possible use the whole URL as a source for connect-src.

Flags: needinfo?(tom)
Attachment #9475774 - Attachment description: Bug 1803607 - [about:logging] Handle uploading errors r=canaltinova → WIP: Bug 1803607 - [about:logging] Handle uploading errors r=canaltinova

(In reply to Tom Schuster from comment #18)

You can use connect-src https: and add the URL about:logging to the sConnectSrcHttpsAllowList. I can also provide a patch that would make it possible use the whole URL as a source for connect-src.

Ah thanks!

Yes I believe it could be good to be able to specify the stricter URL. I can file a bug for that and I'll let you assess the priority.

See Also: → 1957424
Attachment #9475788 - Attachment description: WIP: Bug 1803607 - Allow using https: hosts for connect-src in CSP validation → Bug 1803607 - Allow using https: hosts for connect-src in CSP validation
Attachment #9473408 - Attachment description: WIP: Bug 1803607 - Convert aboutLogging.js to a JS module, and other minor changes r=canaltinova → Bug 1803607 - Convert aboutLogging.js to a JS module, and other minor changes r=canaltinova
Attachment #9475170 - Attachment description: WIP: Bug 1803607 - Move the about:logging code to its own subdirectory r=canaltinova → Bug 1803607 - Move the about:logging code to its own subdirectory r=canaltinova
Attachment #9474113 - Attachment description: WIP: Bug 1803607 - [about:logging] Avoid calling desktop-specific functions on android r=canaltinova → Bug 1803607 - [about:logging] Avoid calling desktop-specific functions on android r=canaltinova
Attachment #9474824 - Attachment description: WIP: Bug 1803607 - [about:logging] Provide a separate UI for Android r=canaltinova → Bug 1803607 - [about:logging] Provide a separate UI for Android r=canaltinova
Attachment #9475460 - Attachment description: WIP: Bug 1803607 - [about:logging] Add a test to cover the new functionality r=canaltinova → Bug 1803607 - [about:logging] Add a test to cover the new functionality r=canaltinova
Attachment #9475774 - Attachment description: WIP: Bug 1803607 - [about:logging] Handle uploading errors r=canaltinova → Bug 1803607 - [about:logging] Handle uploading errors r=canaltinova

Here is a video of the current patch.

Hey Tom, in attachment 9475460 [details], I'm adding a test for this patch. In this test I want to test upload of the profile data. I used https://example.com, but then I had to add an entry for this host to the CSP as well. I don't feel like this is a good solution but I don't see any other.

Is that OK? Would you see an alternative solution for that?

Thanks !

Flags: needinfo?(tschuster)

Good question. I don't really know enough about frontend testing, but what I do know that example.com is a valid domain that we don't control, so we should not allow access to it. Maybe we could add support for firefox.com domain similar to this https://searchfox.org/mozilla-central/source/build/pgo/server-locations.txt#361-362 ?

Flags: needinfo?(tschuster)

It looks like this was easier than expected.
The process isn't well documented: all that's needed is add a line to server-locations.txt then run ./mach python build/pgo/genpgocert.py like mentioned in https://firefox-source-docs.mozilla.org/build/buildsystem/test_certificates.html (but there's no need to add a certspec file! if all you need is adding a new host to the default certificate). => Bug 1958861

Note: I think the mochitest entry is server-locations.txt is broken, at least for "normal" http requests, as the served request doesn't seem to have a certificate that includes this DNS name. I filed Bug 1958854 for this.

Pushed by jwajsberg@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ae1bbf9636b3 Convert aboutLogging.js to a JS module, and other minor changes r=canaltinova https://hg.mozilla.org/integration/autoland/rev/e6f5e8d0bed4 Move the about:logging code to its own subdirectory r=canaltinova,desktop-theme-reviewers,Itiel https://hg.mozilla.org/integration/autoland/rev/30c1813faa67 [about:logging] Avoid calling desktop-specific functions on android r=canaltinova https://hg.mozilla.org/integration/autoland/rev/f08c06c707d7 [about:logging] Provide a separate UI for Android r=canaltinova,fluent-reviewers,desktop-theme-reviewers,profiler-reviewers,flod,dao https://hg.mozilla.org/integration/autoland/rev/2cef71f8e54b [about:logging] Add a test to cover the new functionality r=canaltinova,freddyb https://hg.mozilla.org/integration/autoland/rev/605a43ed5fa8 [about:logging] Handle uploading errors r=canaltinova,fluent-reviewers,flod https://hg.mozilla.org/integration/autoland/rev/2899bb9eeaae Localize about:logging on Android too r=flod,geckoview-reviewers,eemeli,m_kato https://hg.mozilla.org/integration/autoland/rev/70705c6013fa Allow using https: hosts for connect-src in CSP validation r=simonf
Regressions: 1959401
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: